feat(comments): replace the legacy options menu with the post options sheet - #3405
Conversation
… sheet Five comment surfaces never used PostOptionsModal. They fell back to the menu in commentsView, which offered four items: copy link, copy text, open thread, cancel. On botComments, commentsDisplay, commentsModal and the profile waves and comments tabs, a comment had no delete, edit, report, pin, translate, bookmark or moderation action at all. The fallback now mounts PostOptionsModal, so every comment surface gets the same actions the post detail screen already had. Surfaces that pass handleOnOptionsPress (waves) keep routing to their own sheet. Two actions the legacy menu had were missing from the sheet, so they are added rather than dropped: - copy-text copies a plain-text summary, so links and images do not come through as markdown syntax, matching what the old menu did. Offered wherever the content has a body. - open-thread is delegated through a new optional onOpenThread prop and is offered only where a handler is given, since only comment surfaces can open a thread. It defers like the other cases that leave the sheet. The dead legacy handler and its now-unused imports are removed from commentsContainer.
Greptile SummaryThe PR replaces the legacy comment menu with the shared post-options sheet while preserving copy-text, open-thread, and in-place deletion behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported deletion navigation issue is fixed by delegating deletion to the comments container and returning before the modal fallback can navigate back.
|
| Filename | Overview |
|---|---|
| src/components/comments/container/commentsContainer.tsx | Removes the legacy menu callback while retaining the internal comment deletion and thread-opening behavior. |
| src/components/comments/view/commentsView.tsx | Replaces the legacy options modal with PostOptionsModal and correctly adapts deletion to the container callback. |
| src/components/postOptionsModal/container/postOptionsModal.tsx | Adds gated copy-text and open-thread actions while preserving delegated deletion behavior. |
| src/constants/options/post.ts | Registers the two new actions in the shared post-options ordering. |
| src/config/locales/en-US.json | Adds valid English labels for copy-text and open-thread. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User opens comment options] --> B{Custom options handler?}
B -->|Yes| C[Open surface-specific sheet]
B -->|No| D[Open PostOptionsModal]
D --> E{Selected action}
E -->|Copy text| F[Create plain-text summary and copy]
E -->|Open thread| G[Delegate to onOpenThread]
E -->|Delete| H[Delegate to CommentsContainer]
H --> I[Delete comment and update list in place]
Reviews (2): Last reviewed commit: "fix(comments): delegate delete, and scop..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4d22ea3ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <PostOptionsModal | ||
| ref={postOptionsModalRef} | ||
| isVisibleTranslateModal={true} | ||
| onOpenThread={_openReplyThread} |
There was a problem hiding this comment.
When a user selects Delete for an owned, deletable comment on any newly converted list surface, this modal receives no onDelete, so PostOptionsModal falls through to its default deletion path, which calls navigation.goBack() instead of the existing handleDeleteComment. This unexpectedly closes the profile/bot-comments screen and leaves the deleted item in the list state; on the profile Waves tab it also bypasses wavesQuery.deleteWave and therefore its infinite-query cache update. Pass a deletion callback that delegates to the existing comment/wave handler rather than using the modal default.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in a371e95. Both of you flagged this independently and it was the real defect in the PR.
Traced it: _deletePost only delegates when onDelete is present (postOptionsModal.tsx:451); otherwise it runs its own mutation and calls navigation.goBack() at :478. On a profile tab or bot-comments screen that pops the screen the list is embedded in. It also skips the container's in-place list removal, and on waves it bypasses the PostTypes.WAVE branch of _handleDeleteComment that routes through handleCommentDelete.
The list already had the right handler; the sheet just was not given it. commentsView now passes onDelete forwarding to handleDeleteComment with the same five arguments the inline delete button uses (commentView.tsx:317-323), so permlink, parent and root all arrive as they did before:
const _handleDeleteFromMenu = (item) =>
handleDeleteComment(
item.permlink, item.parent_permlink, item.parent_author,
item.root_author, item.root_permlink,
);Worth noting this is exactly the failure mode the existing comment on _deletePost warns about for waves. The comment was there; I did not apply it when adding a new consumer.
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCommentsView now uses PostOptionsModal for comment actions when no external handler is provided. PostOptionsModal adds conditional copy-text and open-thread actions with localization and platform-specific text summarization. ChangesComment options
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/comments/view/commentsView.tsx`:
- Around line 52-53: Update the upvotePopoverRef declaration to call useRef with
an initial null value, and apply the available popover handle type if one is
defined nearby; leave postOptionsModalRef unchanged.
- Around line 177-180: Update the delete eligibility logic used by
PostOptionsModal, specifically _canDeletePost, to safely handle a missing
currentAccount before accessing its name. Preserve deletion behavior for
authenticated accounts while returning an ineligible result for anonymous access
or when no current account exists, including when _openCommentMenu() opens the
modal.
In `@src/components/postOptionsModal/container/postOptionsModal.tsx`:
- Around line 852-860: Update the copy-text case in the post options handler to
await and store the boolean result from writeToClipboard(_body), then schedule
the alert.copied toast only when that result is true. Base the check on the
generated summary rather than _canCopyText’s raw Markdown validation, preserving
the existing clipboard and timer behavior for successful copies.
- Around line 248-252: Update the copy-text eligibility logic in the post
options modal, specifically _canCopyText, to require both comment context via
isComment and content?.markdownBody. Ensure normal posts cannot receive the
copy-text action while preserving copying for comments with a body.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 207114a6-44d8-46fd-8a04-338be0ec63f2
📒 Files selected for processing (5)
src/components/comments/container/commentsContainer.tsxsrc/components/comments/view/commentsView.tsxsrc/components/postOptionsModal/container/postOptionsModal.tsxsrc/config/locales/en-US.jsonsrc/constants/options/post.ts
💤 Files with no reviewable changes (1)
- src/components/comments/container/commentsContainer.tsx
Three review findings. Delete was falling through to the sheet's own path on every converted surface, which calls navigation.goBack() and would pop the profile or bot-comments screen the list is embedded in. It also skipped the container's in-place list removal and, on waves, the wave-specific delete that updates the infinite-query cache. The sheet now receives an onDelete that forwards to the list's existing handler with the same arguments the inline delete button uses. copy-text and open-thread were documented as comment-only but gated only on a body and a handler, so copy-text would have appeared on the post detail sheet too. Both are now scoped to content with a parent, so post detail is unchanged. The copy-text success toast fired on attempt rather than result. writeToClipboard returns false for empty text, and the plain-text summary can be empty where a comment body is only an image or markup, so that path could report a copy that never happened.
…mation Two review findings, both regressions in the previous commit. postScreen renders comments and waves as primary content, so gating navigation.goBack() on parent_author left a comment's own detail screen showing content that had just been deleted. Content shape cannot distinguish 'comment in a list' from 'comment as the screen' - only the caller knows - so the fallback pops unconditionally again and consumers that own a list must pass onDelete. #3407 tracks inverting this into an explicit opt-in, which is the version that fails safe. The delete delegation also asked twice: PostOptionsModal confirms before invoking onDelete, and _handleDeleteComment then confirmed again, so one action needed two confirmations and could be cancelled at the second after being confirmed at the first. Splits that handler in two. _deleteCommentConfirmed mutates without prompting and is what the sheet delegates to; _handleDeleteComment prompts and then calls it, for the inline delete button which has no confirmation of its own. commentsContainer has no confirmation step, so the equivalent path added in #3405 was never affected.
1 participant
Closes #3394
Five comment surfaces never used
PostOptionsModal. They fell back to the menu incommentsView.tsx, which offered four items: copy link, copy text, open thread, cancel.src/screens/botComments/screen/botComments.tsxsrc/components/commentsDisplay/view/commentsDisplayView.tsxsrc/components/commentsModal/container/commentsModal.tsxsrc/components/profile/children/wavesTabContent.tsxsrc/components/profile/children/commentsTabContent.tsxOn those screens a comment had no delete, edit, report, pin, translate, bookmark or moderation action. The fallback now mounts
PostOptionsModal, so they get what the post detail screen already had. Surfaces that passhandleOnOptionsPress(waves) keep routing to their own sheet, unchanged.The two actions that would have been silently dropped
PostOptionsModalhad no equivalent for two of the legacy menu's items, so a straight swap would have quietly removed them. Both are added instead:copy-textcopies a plain-text summary viapostBodySummary, so links and images do not come through as raw markdown, matching the old behaviour exactly. Offered wherever the content has a body.open-threadis delegated through a new optionalonOpenThreadprop and only offered where a handler is given, since only comment surfaces can open a thread. It defers withawait delay(700)like the other cases that leave the sheet.Labels go in
post_dropdownlowercase, matching the block: the sheet uppercases at render.Things worth knowing, not changed here
reblogandcross-poston comments. That is pre-existing: post detail comments already route throughPostOptionsModal, so those have been shown on comments all along. This change makes them visible on five more surfaces. Gating them ondepth === 0would be a behaviour change to the post detail screen too, so it belongs in its own issue rather than riding along here.commentsModalis documented as draft. Its own source note says actions "do not respond as expected since most of action rely on modals and action sheets which causes a conflict". That conflict is unchanged: the legacyOptionsModalwas also anActionSheet, so this swaps one nested sheet for another. Not fixed, not made worse.Cleanup
_handleOnPressCommentMenuand thehandleOnPressCommentMenuprop are removed, along with four imports incommentsContainerthat only it used.Testing
yarn test:ci: 730 passed, 1 skipped, 49 suites.yarn lint: 0 errors.Device checks, ideally on a profile comments tab and the waves tab:
Summary by CodeRabbit